<html>
<head><meta charset="utf-8"><title>Better MSRV testing idea · clippy · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/index.html">clippy</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html">Better MSRV testing idea</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="236199904"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236199904" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236199904">(Apr 26 2021 at 15:59)</a>:</h4>
<p>Currently, our msrv test mostly amounts to "nothing lints at msrv=1.0.0". I have an idea for testing msrv more thoroughly. Suppose a typical lint UI test file starts like this.</p>
<div class="codehilite" data-code-language="Rust"><pre><span></span><code><span class="c1">// test-clippy-msrv</span>
<span class="cp">#![clippy::msrv = </span><span class="s">"1.47.0"</span><span class="cp">]</span><span class="w"></span>
</code></pre></div>
<p>Our test framework would see <code>// test-clippy-msrv</code> and run the UI test a second time. For the second run, any <code>clippy::msrv</code> attribute will be decremented. For this example, this attribute would become <code>#![clippy::msrv="1.46.99"]</code>. And then we simply assert that there are no warnings (stderr is empty). Now we have tested "at msrv has no effect" and "just below msrv disables the lint". The attribute in the test file nicely matches the msrv of the lint. Thoughts?</p>



<a name="236202251"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236202251" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> flip1995 <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236202251">(Apr 26 2021 at 16:15)</a>:</h4>
<p>Wouldn't it be easier to just add the same test twice with different MSRVs? We already do this for some lints. </p>
<p>Also, I don't think we need to test for every lint the edge case that it still lints when the MSRV exactly matches. This is rather a test that MSRV works in general than a test that an MSRV exists for a lint.</p>
<p>I'd rather not patch something Clippy specific in our version of compiletest, as long as the goal is to reuse <code>rustc</code>'s  compiletest sometime in the future.</p>



<a name="236205655"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236205655" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236205655">(Apr 26 2021 at 16:40)</a>:</h4>
<p>I can see how verifying "above and below" msrv for every lint is maybe just not very valuable.</p>



<a name="236205845"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236205845" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236205845">(Apr 26 2021 at 16:41)</a>:</h4>
<p>I'm a little concerned about lints where the msrv is dynamic. For instance, <code>Option::map_or_else</code> and <code>Result::map_or_else</code> have different MSRV. I just noticed that <code>map_unwrap_or</code> lint does not handle this perfectly.</p>



<a name="236206016"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236206016" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236206016">(Apr 26 2021 at 16:43)</a>:</h4>
<p>Actually maybe the lint is fine, but it seems like that could be tested better. Similar scenario with <code>Iterator::copied</code> and <code>Option::copied</code>.</p>



<a name="236210875"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236210875" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> flip1995 <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236210875">(Apr 26 2021 at 17:17)</a>:</h4>
<p>When a lint has 2 MSRVs for different cases, we could handle this like the <code>needless_question_mark</code> lint: <a href="https://github.com/rust-lang/rust-clippy/blob/919a1a40fe4ec1118eece46b1f4c03c311ab8cda/tests/ui/needless_question_mark.fixed#L99-L169">Tests</a>; <a href="https://github.com/rust-lang/rust-clippy/blob/aecccbc579c0092e609f47fabc4d5fb6dbe110d0/clippy_lints/src/needless_question_mark.rs#L66-L67">Code</a></p>
<p>In this case testing all of the MSRV cases might be worth it. But I don't think we have to do this for every lint.</p>



<a name="236213895"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236213895" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236213895">(Apr 26 2021 at 17:36)</a>:</h4>
<p>That looks good</p>



<a name="236214571"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236214571" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236214571">(Apr 26 2021 at 17:41)</a>:</h4>
<p>Why does that lint need msrv?</p>



<a name="236214645"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236214645" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236214645">(Apr 26 2021 at 17:41)</a>:</h4>
<p>msrv should be based on suggestions, not what the lint detects, right?</p>



<a name="236214869"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236214869" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> flip1995 <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236214869">(Apr 26 2021 at 17:43)</a>:</h4>
<p>Oh yeah, you're right. It can only trigger if the <code>?</code> is used. So it can only trigger if it already meets the MSRV.  Good catch!</p>



<a name="236215074"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236215074" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236215074">(Apr 26 2021 at 17:44)</a>:</h4>
<p>One more thought on msrv. I think we should name msrv consts after the rust feature that is used the suggestion, rather than after the lint name.</p>



<a name="236215203"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236215203" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236215203">(Apr 26 2021 at 17:45)</a>:</h4>
<p>Along with that, maybe we could put all those in one mod, and they would be essentially decoupled from lints.</p>



<a name="236215271"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236215271" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> flip1995 <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236215271">(Apr 26 2021 at 17:45)</a>:</h4>
<p>Makes sense, this would allow us to reuse those constants. We may also be able to define them in one place, rather than spreading them across  files.</p>



<a name="236215380"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236215380" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> flip1995 <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236215380">(Apr 26 2021 at 17:46)</a>:</h4>
<p><span class="user-mention silent" data-user-id="360405">Cameron Steffen</span> <a href="#narrow/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea/near/236215203">said</a>:</p>
<blockquote>
<p>Along with that, maybe we could put all those in one mod, and they would be essentially decoupled from lints.</p>
</blockquote>
<p>Good idea <span aria-label="smiley" class="emoji emoji-1f603" role="img" title="smiley">:smiley:</span></p>



<a name="236215484"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236215484" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236215484">(Apr 26 2021 at 17:46)</a>:</h4>
<p>Ya. For example, <code>meets_msrv(msrv, RESULT_MAP_OR_ELSE_MSRV)</code></p>



<a name="236215526"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236215526" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236215526">(Apr 26 2021 at 17:47)</a>:</h4>
<p>Or <code>msrvs::RESULT_MAP_OR_ELSE</code></p>



<a name="236215647"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236215647" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> flip1995 <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236215647">(Apr 26 2021 at 17:47)</a>:</h4>
<p><code>msrvs::RESULT_MAP_OR_ELSE</code> I like this, since it's consistent with the <code>paths</code> module and how we use paths.</p>



<a name="236215897"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236215897" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236215897">(Apr 26 2021 at 17:49)</a>:</h4>
<p>Or <code>msrv!(Result::map_or_else)</code> <span aria-label="laughing" class="emoji emoji-1f606" role="img" title="laughing">:laughing:</span></p>



<a name="236216076"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236216076" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236216076">(Apr 26 2021 at 17:50)</a>:</h4>
<p>(not really)</p>



<a name="236216225"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236216225" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> flip1995 <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236216225">(Apr 26 2021 at 17:51)</a>:</h4>
<p>Oh yeah, let's define a proc macro, that fetches the Rust compiler during compilation time, figures out when the relevant feature was added and inserts the MSRV const automatically. Maybe we'd have to implement a Rust frontend inside the proc macro, but this should be fine, since it'll be easier to define MSRV constants in the future. /s <span aria-label="stuck out tongue closed eyes" class="emoji emoji-1f61d" role="img" title="stuck out tongue closed eyes">:stuck_out_tongue_closed_eyes:</span></p>



<a name="236216422"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236216422" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236216422">(Apr 26 2021 at 17:53)</a>:</h4>
<p>You lost me at "fetches the Rust compiler during compilation time" lol</p>



<a name="236229000"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Better%20MSRV%20testing%20idea/near/236229000" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Better.20MSRV.20testing.20idea.html#236229000">(Apr 26 2021 at 19:21)</a>:</h4>
<p>Done <a href="https://github.com/rust-lang/rust-clippy/issues/7137">rust-lang/rust-clippy#7137</a></p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>